Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

packages/common/src/local-first/Relay.ts:324

• createRelayStorageTables now uses plain create table ... without if not exists. This will throw on subsequent relay starts against an existing database (e.g., apps/relay/src/startBunRelay.ts calls createRelayStorageTables(deps) unconditionally). Restore if not exists here, or make the call sites guard table creation based on DB existence/migrations.

packages/common/src/Task.ts (1)

1742-1751: Zvažte zjednodušení extrakce task argumentu.

IIFE na řádcích 1742-1746 lze zjednodušit na ternární výraz, protože logika je přímočará.

♻️ Návrh zjednodušení

 export function concurrently<T, E, D = unknown>(
   concurrencyOrTask: Concurrency | Task<T, E, D>,
   taskOrFallback?: Task<T, E, D>,
 ): Task<T, E, D> {
   const isTask = isFunction(concurrencyOrTask);
-  const task = (() => {
-    if (isTask) return concurrencyOrTask;
-    assert(taskOrFallback, "Task is required when concurrency is provided.");
-    return taskOrFallback;
-  })();
+  if (!isTask) {
+    assert(taskOrFallback, "Task is required when concurrency is provided.");
+  }
+  const task = isTask ? concurrencyOrTask : taskOrFallback!;

   return Object.assign((run: Run<D>) => run(task), {
     [concurrencyBehaviorSymbol]: isTask ? maxPositiveInt : concurrencyOrTask,
   });
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/common/src/Task.ts` around lines 1742 - 1751, The IIFE used to
derive task can be replaced with a single ternary expression: directly set task
= isTask ? concurrencyOrTask : (assert(taskOrFallback, "Task is required when
concurrency is provided."), taskOrFallback) or otherwise ensure the assertion
runs and task gets taskOrFallback when isTask is false; update the code that
returns Object.assign((run: Run<D>) => run(task), { [concurrencyBehaviorSymbol]:
isTask ? maxPositiveInt : concurrencyOrTask }) to use the simplified task
binding (referencing isTask, concurrencyOrTask, taskOrFallback,
concurrencyBehaviorSymbol, and maxPositiveInt).

packages/react-native/src/Polyfills.ts (1)

37-40: Preferujte readonly args a odstraňte zbytečnou aserci.

args se nikde nemění, takže Array<unknown> zbytečně zužuje veřejný podpis. Promise.resolve(func(...args)) automaticky vrací Promise<Awaited<T>> bez nutnosti explicitní aserce.

♻️ Navržená úprava

-    try?: <T, U extends Array<unknown>>(
+    try?: <T, U extends ReadonlyArray<unknown>>(
       func: (...args: U) => T | PromiseLike<T>,
       ...args: U
     ) => Promise<Awaited<T>>;
@@
-    PromiseStatic.try = <T, U extends Array<unknown>>(
+    PromiseStatic.try = <T, U extends ReadonlyArray<unknown>>(
       func: (...args: U) => T | PromiseLike<T>,
       ...args: U
-    ): Promise<Awaited<T>> =>
-      new Promise<Awaited<T>>((resolve, reject) => {
-        try {
-          resolve(func(...args) as Awaited<T>);
-        } catch (error) {
-          reject(error);
-        }
-      });
+    ): Promise<Awaited<T>> => {
+      try {
+        return Promise.resolve(func(...args));
+      } catch (error) {
+        return Promise.reject(error);
+      }
+    };

Platí pro: 37–40, 58–68

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/react-native/src/Polyfills.ts` around lines 37 - 40, Upravte veřejné
podpisy metody/variantu označené jako "try?" (a obdobné přetížení v rozsahu
58–68) tak, aby používaly readonly typ pro argumenty (např. ...args: readonly
unknown[]) místo mutovatelného Array<unknown>, a odstraňte zbytečnou explicitní
aserci typu při vracení Promise (ponechte Promise.resolve(func(...args)) bez "as
Promise<Awaited<T>>"), čímž zachováte správný návratový typ Awaited<T> a
reflektujete, že args se nemění.

packages/common/test/WebSocket.test.ts (1)

21-42: Sdílený WS server mezi testy zhoršuje izolaci.

Tahle změna zavádí jednu společnou serverovou instanci a globální port pro celý soubor. Když některý test nechá otevřený socket nebo změní serverový stav, další testy už neběží nad čistým prostředím, takže vznikající nestabilitu bude těžké reprodukovat. Pokud chcete zůstat u beforeAll, přidejte aspoň explicitní reset serveru mezi testy.

Based on learnings "Never rely on global state or shared mutable deps between tests. Create fresh deps at the start of each test".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/common/test/WebSocket.test.ts` around lines 21 - 42, The tests share
a single global WebSocket server via the module-level variable port and
beforeAll/afterAll (createServer/closeServer, startWsServer/stopWsServer), which
breaks isolation; change to create and tear down a fresh server for each test
(use beforeEach/afterEach or call an explicit reset between tests) so that each
test invokes createServer/startWsServer and closes it with
closeServer/stopWsServer (or resets server state) and remove reliance on the
global port variable to ensure no leftover sockets or mutated server state leak
between tests.

packages/common/test/Ref.test.ts (1)

11-133: Přidejte i compile-time aserce pro nové Ref API.

Runtime pokrytí je tu dobré, ale hlavní změna API je i typová. Bez expectTypeOf nezachytíte regresi jako set(): void nebo změněný návratový typ u getAndSet / setAndGet / modify.

💡 Možné doplnění

-import { describe, expect, test } from "vitest";
+import { describe, expect, expectTypeOf, test } from "vitest";
 import { createRef } from "../src/Ref.js";

+test("preserves the Ref type contract", () => {
+  const ref = createRef(1);
+
+  expectTypeOf(ref.set(2)).toEqualTypeOf<void>();
+  expectTypeOf(ref.getAndSet(2)).toEqualTypeOf<number>();
+  expectTypeOf(ref.setAndGet(2)).toEqualTypeOf<number>();
+  expectTypeOf(
+    ref.modify((current) => [`value:${current}`, current + 1] as const),
+  ).toEqualTypeOf<string>();
+});
+
 describe("get", () => {

As per coding guidelines "Use `expectTypeOf` from Vitest for compile-time type assertions in tests".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/common/test/Ref.test.ts` around lines 11 - 133, Add compile-time
type assertions using Vitest's expectTypeOf for the new Ref API: import
expectTypeOf and assert types for createRef return and each method signature
(createRef, Ref.set, Ref.get, getAndSet, setAndGet, update, getAndUpdate,
updateAndGet, modify) to ensure set returns void, get returns the state type,
getAndSet/setAndGet/modify return the correct value types, and updater callbacks
accept and return the state type; place these expectTypeOf checks alongside the
runtime tests to catch type regressions.

packages/common/src/local-first/Sync.ts (1)

430-444: Type casting pro kontrolu AbortError.

Type casting (result.error as { type?: string }).type je křehký přístup. Zvažte zavedení type guard funkce pro kontrolu AbortError.

♻️ Návrh type guard funkce

const isAbortError = (error: unknown): boolean =>
  typeof error === "object" &&
  error !== null &&
  "type" in error &&
  (error as { type: unknown }).type === "AbortError";

Pak lze použít:

-if ((result.error as { type?: string }).type !== "AbortError") {
+if (!isAbortError(result.error)) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/common/src/local-first/Sync.ts` around lines 430 - 444, Replace the
fragile cast when checking for an AbortError in the syncRun result handler by
adding a reusable type guard (e.g., isAbortError) and using it instead of
`(result.error as { type?: string }).type`; update the success/error callback in
syncRun where you currently call
config.onError(createUnknownError(result.error)) and the conditional `if
((result.error as { type?: string }).type !== "AbortError")` to use
isAbortError(result.error) to decide whether to call createUnknownError, keeping
the surrounding logic (syncOwnerRefs.has(owner.id),
sendSubscribeForOwner(owner)) unchanged.

packages/common/src/Store.ts (1)

94-99: Nadbytečné volání ref.get() v updateAndGet.

Metoda ref.updateAndGet(updater) již vrací aktualizovaný stav, takže následné volání ref.get() je redundantní. Pro konzistenci s ostatními metodami by stačilo vrátit výsledek přímo.

♻️ Možná optimalizace

     updateAndGet: (updater) => {
       const previousState = ref.get();
-      ref.updateAndGet(updater);
+      const newState = ref.updateAndGet(updater);
       notifyIfChanged(previousState);
-      return ref.get();
+      return newState;
     },

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/common/src/Store.ts` around lines 94 - 99, The updateAndGet
implementation calls ref.updateAndGet(updater) but then redundantly calls
ref.get(); change it to capture and return the value returned by
ref.updateAndGet(updater) instead of calling ref.get(), while still calling
notifyIfChanged(previousState) as before; update the function named updateAndGet
to set something like const newState = ref.updateAndGet(updater);
notifyIfChanged(previousState); return newState so you avoid the extra ref.get()
call.

packages/common/src/local-first/Relay.ts (2)

283-300: ⚠️ Potential issue | 🔴 Critical

deleteOwner musí běžet pod stejným mutexByOwnerId jako zápisy.

writeMessages je teď serializované per ownerId, ale deleteOwner ten samý lock obchází. Paralelní deleteOwner(ownerId) a writeMessages(ownerId, ...) tak pořád můžou smazaného ownera hned částečně obnovit, včetně usage metadat.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/common/src/local-first/Relay.ts` around lines 283 - 300, deleteOwner
currently runs its DB transaction without acquiring the per-owner lock used by
writeMessages; wrap the entire deletion logic (the deps.sqlite.transaction
callback and the call to sqliteStorageBase.deleteOwner) in the same mutex
acquisition used by writeMessages (mutexByOwnerId for the given ownerId) so
deleteOwner(ownerId) is serialized with writeMessages(ownerId,...). Acquire
mutexByOwnerId(ownerId) before starting the transaction and release it after
sqliteStorageBase.deleteOwner completes to prevent concurrent writeMessages from
recreating data.

---

184-190: ⚠️ Potential issue | 🟠 Major

Deduplicitní timestamp odfiltrujte ještě před výpočtem kvóty.

existingTimestampsSet pokrývá jen hodnoty už uložené v DB. Když jeden batch obsahuje stejný timestamp víckrát, všechny duplicitní položky projdou do newMessages, takže se na Lines 205-211 započítají bajty 2× a na Lines 248-253 se navýší usage, i když insert ... on conflict do nothing uloží jen jednu zprávu. To umí vrátit falešný StorageQuotaError.

Možná oprava

-              const existingTimestampsSet = new Set(
+              const seenTimestamps = new Set(
                 existingTimestampsResult.map((t) => t.toString()),
               );
               const newMessages = filterArray(
                 messagesWithTimestampBytes,
-                (m) => !existingTimestampsSet.has(m.timestamp.toString()),
+                (m) => {
+                  const timestamp = m.timestamp.toString();
+                  if (seenTimestamps.has(timestamp)) return false;
+                  seenTimestamps.add(timestamp);
+                  return true;
+                },
               );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/common/src/local-first/Relay.ts` around lines 184 - 190,
Batch-internal duplicate timestamps are not being removed before quota
calculation, causing bytes and usage to be double-counted; deduplicate
messagesWithTimestampBytes by timestamp.toString() first (e.g., produce a
batchUnique list/Set keyed by m.timestamp.toString() that keeps the first
occurrence) and then compute existingTimestampsSet and filter against it to
produce newMessages; ensure you reference the same deduplicated collection when
calculating bytes and incrementing usage (symbols: messagesWithTimestampBytes,
existingTimestampsSet, newMessages, filterArray, StorageQuotaError) so the quota
only accounts for unique timestamps in the incoming batch.

packages/common/src/local-first/Shared.ts (3)

159-161: ⚠️ Potential issue | 🟠 Major

Klíč cache musí zahrnout i appOwner.

Instance se tady znovu používá jen podle name, takže první CreateEvolu bez ownera nebo s jiným ownerem zablokuje pozdější inicializaci pro stejný name. V takovém případě se createSharedEvolu(...) znovu nespustí a transports.addConsumer(...) pro nového ownera vůbec neproběhne.

Also applies to: 183-225

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/common/src/local-first/Shared.ts` around lines 159 - 161, The
cache/mutex key currently uses only Name causing different appOwner instances to
collide; update sharedEvolusByName and sharedEvolusMutexByName to use a
composite key including appOwner (e.g. { name, appOwner } or a stringified
composite) and change the createMutexByKey generic to that composite type so
createSharedEvolu(...) and transports.addConsumer(...) are keyed per owner;
apply the same composite-key change for the other occurrences in the
createSharedEvolu block (around the 183-225 region) so each owner gets its own
mutex and map entry.

---

195-215: ⚠️ Potential issue | 🟠 Major

onDispose může zlikvidovat nově vytvořenou instanci.

Callback z mapy znovu načítá hodnotu jen podle message.name a bez kontroly identity ji disposne i smaže. Pokud mezitím vznikne nová instance pro stejné name, odstraní se omylem ta nová místo původní.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/common/src/local-first/Shared.ts` around lines 195 - 215, The
onDispose callback currently looks up sharedEvolusByName.get(message.name) and
disposes whatever is found, which can accidentally dispose a newly-created
instance; fix by closing over the specific instance created (e.g., capture const
created = maybeSharedEvolu when inserting) and inside
runWithSharedEvoluDeps.daemon/sharedEvolusMutexByName.withLock only call
created[Symbol.asyncDispose]() and delete from sharedEvolusByName if
sharedEvolusByName.get(message.name) === created to ensure identity match (use
the same maybeSharedEvolu reference) before disposing and removing it.

---

332-345: ⚠️ Potential issue | 🟠 Major

Po Dispose může leader dál ukazovat na mrtvý dbWorkerPort.

Chybí vazba mezi evoluPortId a jeho dbWorkerPort, takže dispose vyčistí jen frontu a evoluPort. Když zavřený tab držel leadera a zrovna nebyl na čele fronty, další requesty se budou posílat do zavřeného portu až do timeoutu heartbeat.

Also applies to: 543-570